Skip to content

feat(cli,automation): catch label: 'error' written where type: 'fault' was meant (#3863) - #3893

Merged
os-zhuang merged 1 commit into
mainfrom
claude/fault-edge-label-lint
Jul 28, 2026
Merged

feat(cli,automation): catch label: 'error' written where type: 'fault' was meant (#3863)#3893
os-zhuang merged 1 commit into
mainfrom
claude/fault-edge-label-lint

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Two of the three items left open on #3863. Both make the fault-edge contract legible; neither changes routing behaviour.

1. New lint — flow-error-label-not-fault

type: 'fault' is what routes a failure. label is cosmetic on an ordinary edge. So this, which reads exactly like error handling:

{ source: 'charge_card', target: 'flag_for_review', label: 'error' }

is an ordinary out-edge — and traverseNext runs every unconditional out-edge in parallel (Promise.all). The consequence is the opposite of the intent:

intended actual
node succeeds handler idle handler runs, in parallel with the success path
node fails handler runs handler idle; the run aborts

Silent in both directions — the author believes failures are handled and never notices the handler firing when nothing went wrong. The misreading is especially natural for an AI author, since label: 'error' is precisely what the intent sounds like. That is what makes it worth a build-time diagnostic instead of a puzzled look at a run trace weeks later.

Deliberately narrow, because a label is load-bearing on a branching node: a decision / approval executor returns a branchLabel and traversal then prefers the edge carrying it. Excluded from the rule:

  • edges out of decision / approval / screen / try_catch (the label is a real branch selector there)
  • conditional edges (a guarded path is not the unconditional footgun)
  • edges already typed fault

Matches the obvious synonyms case-insensitively (error, failure, failed, catch, on_error, fault). Verified against the shipped showcase: no findings.

An alias was considered and rejected. Accepting label: 'error' as if it were type: 'fault' would fix the immediate confusion but leave two spellings for one concept, which reads worse than one spelling plus a diagnostic that names the fix.

2. Pinned: a handled failure does not consume a flow-level retry

The two recovery mechanisms have different scopes and must not compound:

Scope On failure
fault edge one node traversal continues from the handler; the run completes
errorHandling: { strategy: 'retry' } the whole flow re-runs from the start

Flow-level retry replays every node that already succeeded — a second notification, a second created record. So a failure a fault edge handled must not also consume a retry, or declaring a handler would silently multiply upstream side effects.

That already held by construction (a routed failure never propagates out of executeNode, so the flow-level catch never fires). It was never asserted. Now a test runs a flow with errorHandling.retry configured plus a fault edge, and checks the upstream node ran exactly once.

Docs

content/docs/automation/flows.mdx and the automation skill gain both points, plus a correction on the edge-property table: label does not select a path except on a branching node. That row previously read as though a label were generally meaningful, which is the belief this lint exists to correct.

Verification

  • packages/cli lint suite: 50 passed (7 new, covering the flag, the synonyms, and all four exclusion cases).
  • packages/services/service-automation: 427 passed, no regressions.
  • objectstack validate on app-showcase: 0 findings from the new rule.
  • ESLint clean; tsc --noEmit reports nothing in lint-flow-patterns.ts; check-role-word gate OK.

Still open on #3863

The third item — a fault edge that loops back. registerFlow's cycle check skips only type === 'back', and type is a single-valued enum, so an edge cannot be both fault and back. That makes "fail → clean up → return to the node and retry" unexpressible today. It needs either the cycle check to treat fault edges as declared back-edges (with a maxRevisions-style bound, since an unbounded fault loop is a new footgun) or type to become composable. That is a behaviour/schema decision and belongs in its own PR.


Generated by Claude Code

…ult'` was meant (#3863)

Two of the three items left open on #3863. Both make the fault-edge contract
legible; neither changes routing behaviour.

New lint `flow-error-label-not-fault`. `type: 'fault'` is what routes a failure;
`label` is cosmetic on an ordinary edge. So an edge written

  { source: 'charge_card', target: 'flag_for_review', label: 'error' }

is an ordinary out-edge, and traverseNext runs every unconditional out-edge in
parallel — the handler fires on every SUCCESSFUL run of the source, alongside
the real success path, and never on a failure. The run still aborts when the
node fails.

Silent both ways: the author believes failures are handled, and never notices
the handler running when nothing went wrong. Especially natural for an AI
author, since the label is exactly what the intent sounds like.

Narrow by construction, because a label IS load-bearing on a branching node — a
decision/approval executor returns a branchLabel and traversal prefers the edge
carrying it. Excluded: edges out of those node types, conditional edges, and
edges already typed 'fault'. Matches the obvious synonyms case-insensitively.
No findings against the shipped showcase.

An alias (accepting label:'error' as type:'fault') was considered and rejected:
two spellings for one concept read worse than one spelling plus a diagnostic
that names the fix.

Also pinned: a handled failure does not consume a flow-level retry. A fault edge
handles one node; errorHandling.retry replays the flow FROM THE START, re-running
everything that already succeeded. The two must not compound. That held by
construction — a routed failure never propagates out of executeNode — and is now
a test so a refactor of the catch path cannot quietly change it.

Docs and the automation skill gain both points, plus a note that `label` does
not select a path except on a branching node.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EdCvGtER9SzJopS4Yh3Pa1
@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 28, 2026 3:05pm

Request Review

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Jul 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cli, packages/services.

22 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/skills-reference.mdx (via packages/cli)
  • content/docs/api/client-sdk.mdx (via @objectstack/cli)
  • content/docs/api/data-flow.mdx (via @objectstack/cli)
  • content/docs/api/environment-routing.mdx (via @objectstack/cli)
  • content/docs/api/error-catalog.mdx (via @objectstack/cli)
  • content/docs/automation/hook-bodies.mdx (via packages/cli)
  • content/docs/automation/webhooks.mdx (via packages/services)
  • content/docs/deployment/backup-restore.mdx (via @objectstack/cli)
  • content/docs/deployment/cli.mdx (via @objectstack/cli)
  • content/docs/deployment/self-hosting.mdx (via @objectstack/cli)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/cli)
  • content/docs/kernel/runtime-services/audit-service.mdx (via packages/services)
  • content/docs/kernel/runtime-services/data-service.mdx (via packages/cli)
  • content/docs/kernel/runtime-services/index.mdx (via packages/cli, packages/services)
  • content/docs/kernel/runtime-services/settings-service.mdx (via packages/services)
  • content/docs/permissions/authentication.mdx (via @objectstack/cli)
  • content/docs/plugins/packages.mdx (via @objectstack/cli, packages/services)
  • content/docs/protocol/kernel/i18n-standard.mdx (via packages/services)
  • content/docs/protocol/kernel/plugin-spec.mdx (via @objectstack/cli)
  • content/docs/protocol/kernel/realtime-protocol.mdx (via @objectstack/cli)
  • content/docs/releases/implementation-status.mdx (via @objectstack/cli)
  • content/docs/releases/v16.mdx (via @objectstack/cli)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review July 28, 2026 15:09
@os-zhuang
os-zhuang merged commit 7ef20d0 into main Jul 28, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/fault-edge-label-lint branch July 28, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants